[VPlan] Use scalar VPPhi instead of VPWidenPHIRecipe in createPlainCFG. #150847

fhahn · 2025-07-27T19:17:50Z

The initial VPlan closely reflects the original scalar loop, so unsing VPWidenPHIRecipe here is premature. Widened phi recipes should only be introduced together with other widened recipes.

llvmbot · 2025-07-27T19:18:20Z

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

The initial VPlan closely reflects the original scalar loop, so unsing VPWidenPHIRecipe here is premature. Widened phi recipes should only be introduced together with other widened recipes.

Full diff: https://github.com/llvm/llvm-project/pull/150847.diff

7 Files Affected:

(modified) llvm/lib/Transforms/Vectorize/LoopVectorize.cpp (+1-1)
(modified) llvm/lib/Transforms/Vectorize/VPlan.h (+13-1)
(modified) llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp (+6-8)
(modified) llvm/lib/Transforms/Vectorize/VPlanPredicator.cpp (+3-3)
(modified) llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp (+13-10)
(modified) llvm/test/Transforms/LoopVectorize/outer-loop-vec-phi-predecessor-order.ll (+1-1)
(modified) llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll (+2-2)

diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 6616e61f9bb84..b6f261a1b3cbc 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -8237,7 +8237,7 @@ VPRecipeBase *VPRecipeBuilder::tryToCreateWidenRecipe(VPSingleDefRecipe *R,
   VPRecipeBase *Recipe;
   Instruction *Instr = R->getUnderlyingInstr();
   SmallVector<VPValue *, 4> Operands(R->operands());
-  if (auto *PhiR = dyn_cast<VPWidenPHIRecipe>(R)) {
+  if (auto *PhiR = dyn_cast<VPPhi>(R)) {
     VPBasicBlock *Parent = PhiR->getParent();
     [[maybe_unused]] VPRegionBlock *LoopRegionOf =
         Parent->getEnclosingLoopRegion();
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index a5de5933d5ff1..55930da0de1dc 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1243,8 +1243,20 @@ struct LLVM_ABI_FOR_TEST VPPhi : public VPInstruction, public VPPhiAccessors {
     return R && R->getOpcode() == Instruction::PHI;
   }
 
+  static inline bool classof(const VPValue *V) {
+    auto *R = dyn_cast<VPInstruction>(V);
+    return R && R->getOpcode() == Instruction::PHI;
+  }
+
+  static inline bool classof(const VPSingleDefRecipe *R) {
+    auto *VPI = dyn_cast<VPInstruction>(R);
+    return VPI && VPI->getOpcode() == Instruction::PHI;
+  }
+
   VPPhi *clone() override {
-    return new VPPhi(operands(), getDebugLoc(), getName());
+    auto *PhiR = new VPPhi(operands(), getDebugLoc(), getName());
+    PhiR->setUnderlyingValue(getUnderlyingValue());
+    return PhiR;
   }
 
   void execute(VPTransformState &State) override;
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 6c1f53b4eaa24..bd73e502768f9 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -91,17 +91,15 @@ void PlainCFGBuilder::fixHeaderPhis() {
   for (auto *Phi : PhisToFix) {
     assert(IRDef2VPValue.count(Phi) && "Missing VPInstruction for PHINode.");
     VPValue *VPVal = IRDef2VPValue[Phi];
-    assert(isa<VPWidenPHIRecipe>(VPVal) &&
-           "Expected WidenPHIRecipe for phi node.");
-    auto *VPPhi = cast<VPWidenPHIRecipe>(VPVal);
-    assert(VPPhi->getNumOperands() == 0 &&
-           "Expected VPInstruction with no operands.");
+    assert(isa<VPPhi>(VPVal) && "Expected VPPhi for phi node.");
+    auto *PhiR = cast<VPPhi>(VPVal);
+    assert(PhiR->getNumOperands() == 0 && "Expected VPPhi with no operands.");
     assert(isHeaderBB(Phi->getParent(), LI->getLoopFor(Phi->getParent())) &&
            "Expected Phi in header block.");
     assert(Phi->getNumOperands() == 2 &&
            "header phi must have exactly 2 operands");
     for (BasicBlock *Pred : predecessors(Phi->getParent()))
-      VPPhi->addOperand(
+      PhiR->addOperand(
           getOrCreateVPOperand(Phi->getIncomingValueForBlock(Pred)));
   }
 }
@@ -207,8 +205,8 @@ void PlainCFGBuilder::createVPInstructionsForVPBB(VPBasicBlock *VPBB,
       // Phi node's operands may have not been visited at this point. We create
       // an empty VPInstruction that we will fix once the whole plain CFG has
       // been built.
-      NewR = new VPWidenPHIRecipe(Phi, nullptr, Phi->getDebugLoc(), "vec.phi");
-      VPBB->appendRecipe(NewR);
+      NewR = VPIRBuilder.createScalarPhi({}, Phi->getDebugLoc(), "vec.phi");
+      NewR->setUnderlyingValue(Phi);
       if (isHeaderBB(Phi->getParent(), LI->getLoopFor(Phi->getParent()))) {
         // Header phis need to be fixed after the VPBB for the latch has been
         // created.
diff --git a/llvm/lib/Transforms/Vectorize/VPlanPredicator.cpp b/llvm/lib/Transforms/Vectorize/VPlanPredicator.cpp
index 3b3bbc312402e..862b9301e8ca5 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanPredicator.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanPredicator.cpp
@@ -227,10 +227,10 @@ void VPPredicator::createSwitchEdgeMasks(VPInstruction *SI) {
 }
 
 void VPPredicator::convertPhisToBlends(VPBasicBlock *VPBB) {
-  SmallVector<VPWidenPHIRecipe *> Phis;
+  SmallVector<VPPhi *> Phis;
   for (VPRecipeBase &R : VPBB->phis())
-    Phis.push_back(cast<VPWidenPHIRecipe>(&R));
-  for (VPWidenPHIRecipe *PhiR : Phis) {
+    Phis.push_back(cast<VPPhi>(&R));
+  for (VPPhi *PhiR : Phis) {
     // The non-header Phi is converted into a Blend recipe below,
     // so we don't have to worry about the insertion order and we can just use
     // the builder. At this point we generate the predication tree. There may
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 935a4e41bab06..32b8b4f887b47 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -63,17 +63,20 @@ bool VPlanTransforms::tryToConvertVPInstructionsToVPRecipes(
       Instruction *Inst = cast<Instruction>(VPV->getUnderlyingValue());
 
       VPRecipeBase *NewRecipe = nullptr;
-      if (auto *VPPhi = dyn_cast<VPWidenPHIRecipe>(&Ingredient)) {
-        auto *Phi = cast<PHINode>(VPPhi->getUnderlyingValue());
+      if (auto *PhiR = dyn_cast<VPPhi>(&Ingredient)) {
+        auto *Phi = cast<PHINode>(PhiR->getUnderlyingValue());
         const auto *II = GetIntOrFpInductionDescriptor(Phi);
-        if (!II)
-          continue;
-
-        VPValue *Start = Plan->getOrAddLiveIn(II->getStartValue());
-        VPValue *Step =
-            vputils::getOrCreateVPValueForSCEVExpr(*Plan, II->getStep(), SE);
-        NewRecipe = new VPWidenIntOrFpInductionRecipe(
-            Phi, Start, Step, &Plan->getVF(), *II, Ingredient.getDebugLoc());
+        if (!II) {
+          NewRecipe = new VPWidenPHIRecipe(Phi, nullptr, PhiR->getDebugLoc());
+          for (VPValue *Op : PhiR->operands())
+            NewRecipe->addOperand(Op);
+        } else {
+          VPValue *Start = Plan->getOrAddLiveIn(II->getStartValue());
+          VPValue *Step =
+              vputils::getOrCreateVPValueForSCEVExpr(*Plan, II->getStep(), SE);
+          NewRecipe = new VPWidenIntOrFpInductionRecipe(
+              Phi, Start, Step, &Plan->getVF(), *II, Ingredient.getDebugLoc());
+        }
       } else {
         assert(isa<VPInstruction>(&Ingredient) &&
                "only VPInstructions expected here");
diff --git a/llvm/test/Transforms/LoopVectorize/outer-loop-vec-phi-predecessor-order.ll b/llvm/test/Transforms/LoopVectorize/outer-loop-vec-phi-predecessor-order.ll
index 1cf410c359f06..32b1fc4455d37 100644
--- a/llvm/test/Transforms/LoopVectorize/outer-loop-vec-phi-predecessor-order.ll
+++ b/llvm/test/Transforms/LoopVectorize/outer-loop-vec-phi-predecessor-order.ll
@@ -35,7 +35,7 @@ define void @test(ptr %src, i64 %n) {
 ; CHECK-NEXT:    [[TMP3:%.*]] = icmp eq <4 x i64> [[TMP2]], [[BROADCAST_SPLAT]]
 ; CHECK-NEXT:    [[TMP4:%.*]] = extractelement <4 x i1> [[TMP3]], i32 0
 ; CHECK-NEXT:    br i1 [[TMP4]], label [[LOOP_2_LATCH4]], label [[LOOP_32]]
-; CHECK:       loop.2.latch4:
+; CHECK:       loop.2.latch3:
 ; CHECK-NEXT:    [[TMP5]] = add nuw nsw <4 x i64> [[VEC_PHI]], splat (i64 1)
 ; CHECK-NEXT:    [[TMP6:%.*]] = icmp eq <4 x i64> [[TMP5]], [[BROADCAST_SPLAT]]
 ; CHECK-NEXT:    [[TMP7:%.*]] = extractelement <4 x i1> [[TMP6]], i32 0
diff --git a/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll b/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll
index 6804817c402bd..20676f3702294 100644
--- a/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll
+++ b/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll
@@ -13,14 +13,14 @@ define void @foo(i64 %n) {
 ; CHECK-NEXT: Successor(s): outer.header
 ; CHECK-EMPTY:
 ; CHECK-NEXT: outer.header:
-; CHECK-NEXT:   WIDEN-PHI ir<%outer.iv> = phi [ ir<%outer.iv.next>, outer.latch ], [ ir<0>, ir-bb<entry> ]
+; CHECK-NEXT:   EMIT-SCALAR ir<%outer.iv> = phi [ ir<%outer.iv.next>, outer.latch ], [ ir<0>, ir-bb<entry> ]
 ; CHECK-NEXT:   EMIT ir<%gep.1> = getelementptr ir<@arr2>, ir<0>, ir<%outer.iv>
 ; CHECK-NEXT:   EMIT store ir<%outer.iv>, ir<%gep.1>
 ; CHECK-NEXT:   EMIT ir<%add> = add ir<%outer.iv>, ir<%n>
 ; CHECK-NEXT: Successor(s): inner
 ; CHECK-EMPTY:
 ; CHECK-NEXT: inner:
-; CHECK-NEXT:   WIDEN-PHI ir<%inner.iv> = phi [ ir<%inner.iv.next>, inner ], [ ir<0>, outer.header ]
+; CHECK-NEXT:   EMIT-SCALAR ir<%inner.iv> = phi [ ir<%inner.iv.next>, inner ], [ ir<0>, outer.header ]
 ; CHECK-NEXT:   EMIT ir<%gep.2> = getelementptr ir<@arr>, ir<0>, ir<%inner.iv>, ir<%outer.iv>
 ; CHECK-NEXT:   EMIT store ir<%add>, ir<%gep.2>
 ; CHECK-NEXT:   EMIT ir<%inner.iv.next> = add ir<%inner.iv>, ir<1>

The initial VPlan closely reflects the original scalar loop, so unsing VPWidenPHIRecipe here is premature. Widened phi recipes should only be introduced together with other widened recipes.

ayalz

LGTM, adding minor comments.

ayalz · 2025-08-04T12:01:58Z

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -8239,7 +8239,7 @@ VPRecipeBase *VPRecipeBuilder::tryToCreateWidenRecipe(VPSingleDefRecipe *R,
  VPRecipeBase *Recipe;
  Instruction *Instr = R->getUnderlyingInstr();
  SmallVector<VPValue *, 4> Operands(R->operands());
-  if (auto *PhiR = dyn_cast<VPWidenPHIRecipe>(R)) {
+  if (auto *PhiR = dyn_cast<VPPhi>(R)) {


Suggested change

if (auto *PhiR = dyn_cast<VPPhi>(R)) {

assert(!isa<VPWidenPHIRecipe>(R) && "Only scalar recipes expected);

if (auto *PhiR = dyn_cast<VPPhi>(R)) {

?

Yep, added an assert to the else branch using !VPRecipeBase::isPhi()

ayalz · 2025-08-04T12:06:47Z

llvm/lib/Transforms/Vectorize/VPlan.h

+  static inline bool classof(const VPSingleDefRecipe *R) {
+    auto *VPI = dyn_cast<VPInstruction>(R);
+    return VPI && VPI->getOpcode() == Instruction::PHI;


Suggested change

static inline bool classof(const VPSingleDefRecipe *R) {

auto *VPI = dyn_cast<VPInstruction>(R);

return VPI && VPI->getOpcode() == Instruction::PHI;

static inline bool classof(const VPSingleDefRecipe *SDR) {

auto *R = dyn_cast<VPInstruction>(SDR);

return R && R->getOpcode() == Instruction::PHI;

consistency w/ the above.

Updated to use VPI consistently, thanks

ayalz · 2025-08-04T12:15:32Z

llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp

@@ -207,8 +205,8 @@ void PlainCFGBuilder::createVPInstructionsForVPBB(VPBasicBlock *VPBB,
      // Phi node's operands may have not been visited at this point. We create


independent nit:

Suggested change

// Phi node's operands may have not been visited at this point. We create

// Phi node's operands may not have been visited at this point. We create

Fixed thanks

ayalz · 2025-08-04T15:18:01Z

llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp

@@ -207,8 +205,8 @@ void PlainCFGBuilder::createVPInstructionsForVPBB(VPBasicBlock *VPBB,
      // Phi node's operands may have not been visited at this point. We create
      // an empty VPInstruction that we will fix once the whole plain CFG has
      // been built.
-      NewR = new VPWidenPHIRecipe(Phi, nullptr, Phi->getDebugLoc(), "vec.phi");
-      VPBB->appendRecipe(NewR);
+      NewR = VPIRBuilder.createScalarPhi({}, Phi->getDebugLoc(), "vec.phi");


Independent: should createScalarPhi() be called createVPPhi() or should VPPhi be called ScalarPhi?

For now it is scalar-phi only, but the distinction could go away in the later stages, if we decide to encode single-scalar/wide in for some recipes.

Ok, may be good to clarify/document the distinction between VPWidenPHIRecipe and VPPhi - the former holds for VF>1 and/or UF>1, the latter holds for scalar, VF=UF=1, currently?

Would be good to resolve the discrepancy between createScalarPhi and VPPhi, one way or another, if they continue to be synonymous.

Will do. To clarify, VPPhi will also be used for scalar phis that remain scalar even with VF > 1 (at least when lowering for exexcute)

ayalz · 2025-08-04T15:25:18Z

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

@@ -63,17 +63,20 @@ bool VPlanTransforms::tryToConvertVPInstructionsToVPRecipes(
      Instruction *Inst = cast<Instruction>(VPV->getUnderlyingValue());

      VPRecipeBase *NewRecipe = nullptr;
-      if (auto *VPPhi = dyn_cast<VPWidenPHIRecipe>(&Ingredient)) {
-        auto *Phi = cast<PHINode>(VPPhi->getUnderlyingValue());
+      if (auto *PhiR = dyn_cast<VPPhi>(&Ingredient)) {


Independent: Ingredient usually stands for an underlying IR Instruction rather than recipe.

Yep, will do separately, thanks

…in-initial-vplan

ayalz · 2025-08-05T05:46:34Z

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -8304,6 +8304,8 @@ VPRecipeBase *VPRecipeBuilder::tryToCreateWidenRecipe(VPSingleDefRecipe *R,
    // Add backedge value.
    PhiRecipe->addOperand(Operands[1]);
    return PhiRecipe;
+  } else {


Else redundant after early exit?

Ah yes, removed the else, thanks

ayalz · 2025-08-05T05:48:20Z

llvm/lib/Transforms/Vectorize/VPlan.h

+    auto *VPI = dyn_cast<VPInstruction>(V);
+    return VPI && VPI->getOpcode() == Instruction::PHI;


Also holds for above classof, for consistency.

Updated to also clean up, thanks

ayalz · 2025-08-05T06:05:08Z

llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp

@@ -207,8 +205,8 @@ void PlainCFGBuilder::createVPInstructionsForVPBB(VPBasicBlock *VPBB,
      // Phi node's operands may have not been visited at this point. We create
      // an empty VPInstruction that we will fix once the whole plain CFG has
      // been built.
-      NewR = new VPWidenPHIRecipe(Phi, nullptr, Phi->getDebugLoc(), "vec.phi");
-      VPBB->appendRecipe(NewR);
+      NewR = VPIRBuilder.createScalarPhi({}, Phi->getDebugLoc(), "vec.phi");


Ok, may be good to clarify/document the distinction between VPWidenPHIRecipe and VPPhi - the former holds for VF>1 and/or UF>1, the latter holds for scalar, VF=UF=1, currently?

Would be good to resolve the discrepancy between createScalarPhi and VPPhi, one way or another, if they continue to be synonymous.

…in-initial-vplan

…eatePlainCFG. (#150847) The initial VPlan closely reflects the original scalar loop, so unsing VPWidenPHIRecipe here is premature. Widened phi recipes should only be introduced together with other widened recipes. PR: llvm/llvm-project#150847

…G. (llvm#150847) The initial VPlan closely reflects the original scalar loop, so unsing VPWidenPHIRecipe here is premature. Widened phi recipes should only be introduced together with other widened recipes. PR: llvm#150847

fhahn requested review from rengolin, ayalz and aniragil July 27, 2025 19:17

llvmbot added vectorizers llvm:transforms labels Jul 27, 2025

[VPlan] Use scalar VPPhi instead of VPWidenPHIRecipe in createPlainCFG.

b2940a8

The initial VPlan closely reflects the original scalar loop, so unsing VPWidenPHIRecipe here is premature. Widened phi recipes should only be introduced together with other widened recipes.

fhahn force-pushed the vplan-use-scalar-phi-in-initial-vplan branch from 32b433c to b2940a8 Compare July 29, 2025 09:43

ayalz approved these changes Aug 4, 2025

View reviewed changes

fhahn added 2 commits August 4, 2025 19:56

Merge remote-tracking branch 'origin/main' into vplan-use-scalar-phi-…

051bf6f

…in-initial-vplan

!fixup address comments, thanks

9d58f06

ayalz reviewed Aug 5, 2025

View reviewed changes

fhahn added 2 commits August 5, 2025 13:34

Merge remote-tracking branch 'origin/main' into vplan-use-scalar-phi-…

62ce5ea

…in-initial-vplan

!fixup address comments, thanks

3f2f9f6

fhahn merged commit e80e7e7 into llvm:main Aug 6, 2025
9 checks passed

fhahn deleted the vplan-use-scalar-phi-in-initial-vplan branch August 6, 2025 13:43

	if (auto *PhiR = dyn_cast<VPPhi>(R)) {
	assert(!isa<VPWidenPHIRecipe>(R) && "Only scalar recipes expected);
	if (auto *PhiR = dyn_cast<VPPhi>(R)) {

		@@ -207,8 +205,8 @@ void PlainCFGBuilder::createVPInstructionsForVPBB(VPBasicBlock *VPBB,
		// Phi node's operands may have not been visited at this point. We create

	// Phi node's operands may have not been visited at this point. We create
	// Phi node's operands may not have been visited at this point. We create

		auto *VPI = dyn_cast<VPInstruction>(V);
		return VPI && VPI->getOpcode() == Instruction::PHI;

[VPlan] Use scalar VPPhi instead of VPWidenPHIRecipe in createPlainCFG. #150847

[VPlan] Use scalar VPPhi instead of VPWidenPHIRecipe in createPlainCFG. #150847

Uh oh!

Conversation

fhahn commented Jul 27, 2025

Uh oh!

llvmbot commented Jul 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ayalz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ayalz Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ayalz Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Jul 27, 2025 •

edited

Loading

ayalz Aug 5, 2025 •

edited

Loading

ayalz Aug 5, 2025 •

edited

Loading